-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement latched topics on TCP #2
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR! If I understand correctly, your are supporting latching only for the TCP transport. In that case, I do not understand the need for the complex "latch helper" setup. I would suggest to re-send latched messages whenever |
Oh man, could it be that simple? I admit I haven't understood the whole relay code, but it looked impossible to me to tell when the client has connected. But as you pointed me to the right direction, it now seems clear to me. Few questions arise:
|
I guess so. At the very least we should prevent the inner
I have never seen a use case for latched topics over UDP. Typically, UDP data is streaming data, where latching makes no sense. Or do you have an actual use case for that? |
Ah, and there is one more issue: If there are no messages to transmit, |
Ok, I'm trying to rewrite and will send a new PR in a while. Your anti-looping suggestion sounds good to me. And regarding latched UDP topics - it probably really doesn't make sense to latch UDP topics... Should I add a warning to the UDP transport when it finds a topic which requires latching? |
Well, now I think if this wasn't my original issue... I kind of get lost thinking about all the possible cases. Wouldn't periodical connection check be a problem? Either performance-wise, or just for the case that the receiver is stopped and is not mentioned to be re-run? (e.g. unsubscribing from all topics on the receiving side). |
Wait, is the dropped connection really a problem? The receiver also sets the "latched" flag at the publishers it creates. So even if the connection drops, the subscribers on the receiving side will get properly handled. Or do I miss some other important point? |
Transmitting a latched topic over the nimbro relay was not supported.
This PR adds several features:
This PR almost surely breaks ABI, if take care about it. On the other hand, API changes are not breaking compatibility with previous versions.